utils: handle flash_attn missing from importlib packages_distributions without crashing - #45524
Conversation
…not in the distribution map
is_flash_attn_2_available / _3 / _4 / _greater_or_equal do two checks:
is_available, _ = _is_package_available("flash_attn", return_version=True)
is_available = is_available and "flash-attn" in [
pkg.replace("_", "-") for pkg in PACKAGE_DISTRIBUTION_MAPPING["flash_attn"]
]
Step 1 uses importlib.util.find_spec, which returns a spec if any
"flash_attn" import is findable (an editable install, a namespace
package, a bundled shim, or a stub module under another project).
Step 2 then assumes that every findable import name also has an entry
in importlib.metadata.packages_distributions().
That assumption does not hold. On Python 3.13 with ComfyUI setups
(huggingface#45520), and in any environment where the import is resolvable via a
non-pip source, packages_distributions() has no "flash_attn" key.
Because the list comprehension is evaluated before the `in` operator,
short-circuit evaluation of the outer `and` does not protect us - the
KeyError fires during `transformers` import and takes down the whole
process before any model is loaded.
Swap the four raising subscripts for `.get(name, [])`. If the name is
missing from the distribution map we simply conclude that the requested
flash-attention flavour is not properly installed - which is the same
answer is_flash_attn_*_available() would have returned anyway - instead
of raising. The inner helper `_is_package_available` already wraps the
same subscript in a try/except, so we are only making the outer call
sites match that contract.
Fixes huggingface#45520
|
cc @vasqu |
vasqu
left a comment
There was a problem hiding this comment.
Thank you, seems valid in either case. Can you
- Update the same in modeling flash attn utils
transformers/src/transformers/modeling_flash_attention_utils.py
Lines 73 to 109 in 57f9936
- Add a small test similar to
transformers/tests/utils/test_modeling_utils.py
Line 2840 in 57f9936
…gression tests Signed-off-by: say <say.apm35@gmail.com>
|
Applied both requests:
|
Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
| assert modeling_auto.__name__ == "transformers.models.auto.modeling_auto" | ||
|
|
||
|
|
||
| class TestFlashAttnDistributionMapMissing(unittest.TestCase): |
There was a problem hiding this comment.
Let's remove these tests please and add a test to modeling utils, right below
transformers/tests/utils/test_modeling_utils.py
Line 2840 in 57f9936
Where we check that no error is there anymore
…ng_utils Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
|
Moved the distribution-map regression test to |
|
Both failing jobs are pre-existing flakes unrelated to this change. tests_processors: 7 tests fail with tests_tensor_parallel_ci: 2 tests fail with |
…s without crashing (huggingface#45524) * utils: stop crashing with KeyError when flash_attn is importable but not in the distribution map is_flash_attn_2_available / _3 / _4 / _greater_or_equal do two checks: is_available, _ = _is_package_available("flash_attn", return_version=True) is_available = is_available and "flash-attn" in [ pkg.replace("_", "-") for pkg in PACKAGE_DISTRIBUTION_MAPPING["flash_attn"] ] Step 1 uses importlib.util.find_spec, which returns a spec if any "flash_attn" import is findable (an editable install, a namespace package, a bundled shim, or a stub module under another project). Step 2 then assumes that every findable import name also has an entry in importlib.metadata.packages_distributions(). That assumption does not hold. On Python 3.13 with ComfyUI setups (huggingface#45520), and in any environment where the import is resolvable via a non-pip source, packages_distributions() has no "flash_attn" key. Because the list comprehension is evaluated before the `in` operator, short-circuit evaluation of the outer `and` does not protect us - the KeyError fires during `transformers` import and takes down the whole process before any model is loaded. Swap the four raising subscripts for `.get(name, [])`. If the name is missing from the distribution map we simply conclude that the requested flash-attention flavour is not properly installed - which is the same answer is_flash_attn_*_available() would have returned anyway - instead of raising. The inner helper `_is_package_available` already wraps the same subscript in a try/except, so we are only making the outer call sites match that contract. Fixes huggingface#45520 * utils: fix same KeyError in modeling_flash_attention_utils and add regression tests Signed-off-by: say <say.apm35@gmail.com> * style: fix ruff formatting in flash_attn fix and tests Signed-off-by: Sai Asish Y <say.apm35@gmail.com> * test: move flash_attn distribution-map regression test to test_modeling_utils Signed-off-by: Sai Asish Y <say.apm35@gmail.com> * revert this * is gh alright? --------- Signed-off-by: say <say.apm35@gmail.com> Signed-off-by: Sai Asish Y <say.apm35@gmail.com> Co-authored-by: SAY-5 <SAY-5@users.noreply.github.com> Co-authored-by: vasqu <antonprogamer@gmail.com> Co-authored-by: Anton Vlasjuk <73884904+vasqu@users.noreply.github.com>
…s without crashing (huggingface#45524) * utils: stop crashing with KeyError when flash_attn is importable but not in the distribution map is_flash_attn_2_available / _3 / _4 / _greater_or_equal do two checks: is_available, _ = _is_package_available("flash_attn", return_version=True) is_available = is_available and "flash-attn" in [ pkg.replace("_", "-") for pkg in PACKAGE_DISTRIBUTION_MAPPING["flash_attn"] ] Step 1 uses importlib.util.find_spec, which returns a spec if any "flash_attn" import is findable (an editable install, a namespace package, a bundled shim, or a stub module under another project). Step 2 then assumes that every findable import name also has an entry in importlib.metadata.packages_distributions(). That assumption does not hold. On Python 3.13 with ComfyUI setups (huggingface#45520), and in any environment where the import is resolvable via a non-pip source, packages_distributions() has no "flash_attn" key. Because the list comprehension is evaluated before the `in` operator, short-circuit evaluation of the outer `and` does not protect us - the KeyError fires during `transformers` import and takes down the whole process before any model is loaded. Swap the four raising subscripts for `.get(name, [])`. If the name is missing from the distribution map we simply conclude that the requested flash-attention flavour is not properly installed - which is the same answer is_flash_attn_*_available() would have returned anyway - instead of raising. The inner helper `_is_package_available` already wraps the same subscript in a try/except, so we are only making the outer call sites match that contract. Fixes huggingface#45520 * utils: fix same KeyError in modeling_flash_attention_utils and add regression tests Signed-off-by: say <say.apm35@gmail.com> * style: fix ruff formatting in flash_attn fix and tests Signed-off-by: Sai Asish Y <say.apm35@gmail.com> * test: move flash_attn distribution-map regression test to test_modeling_utils Signed-off-by: Sai Asish Y <say.apm35@gmail.com> * revert this * is gh alright? --------- Signed-off-by: say <say.apm35@gmail.com> Signed-off-by: Sai Asish Y <say.apm35@gmail.com> Co-authored-by: SAY-5 <SAY-5@users.noreply.github.com> Co-authored-by: vasqu <antonprogamer@gmail.com> Co-authored-by: Anton Vlasjuk <73884904+vasqu@users.noreply.github.com>
…s without crashing (huggingface#45524) * utils: stop crashing with KeyError when flash_attn is importable but not in the distribution map is_flash_attn_2_available / _3 / _4 / _greater_or_equal do two checks: is_available, _ = _is_package_available("flash_attn", return_version=True) is_available = is_available and "flash-attn" in [ pkg.replace("_", "-") for pkg in PACKAGE_DISTRIBUTION_MAPPING["flash_attn"] ] Step 1 uses importlib.util.find_spec, which returns a spec if any "flash_attn" import is findable (an editable install, a namespace package, a bundled shim, or a stub module under another project). Step 2 then assumes that every findable import name also has an entry in importlib.metadata.packages_distributions(). That assumption does not hold. On Python 3.13 with ComfyUI setups (huggingface#45520), and in any environment where the import is resolvable via a non-pip source, packages_distributions() has no "flash_attn" key. Because the list comprehension is evaluated before the `in` operator, short-circuit evaluation of the outer `and` does not protect us - the KeyError fires during `transformers` import and takes down the whole process before any model is loaded. Swap the four raising subscripts for `.get(name, [])`. If the name is missing from the distribution map we simply conclude that the requested flash-attention flavour is not properly installed - which is the same answer is_flash_attn_*_available() would have returned anyway - instead of raising. The inner helper `_is_package_available` already wraps the same subscript in a try/except, so we are only making the outer call sites match that contract. Fixes huggingface#45520 * utils: fix same KeyError in modeling_flash_attention_utils and add regression tests Signed-off-by: say <say.apm35@gmail.com> * style: fix ruff formatting in flash_attn fix and tests Signed-off-by: Sai Asish Y <say.apm35@gmail.com> * test: move flash_attn distribution-map regression test to test_modeling_utils Signed-off-by: Sai Asish Y <say.apm35@gmail.com> * revert this * is gh alright? --------- Signed-off-by: say <say.apm35@gmail.com> Signed-off-by: Sai Asish Y <say.apm35@gmail.com> Co-authored-by: SAY-5 <SAY-5@users.noreply.github.com> Co-authored-by: vasqu <antonprogamer@gmail.com> Co-authored-by: Anton Vlasjuk <73884904+vasqu@users.noreply.github.com>
Fixes #45520.
is_flash_attn_2_available,is_flash_attn_3_available,is_flash_attn_4_available, andis_flash_attn_greater_or_equalall do two checks:Step 1 uses
importlib.util.find_spec, which returns a spec if anyflash_attnimport is findable, an editable install, a namespace package, a bundled shim, or a stub module sitting under another project. Step 2 then assumes that every findable import name also has an entry inimportlib.metadata.packages_distributions().That assumption does not hold in practice. On Python 3.13 with ComfyUI setups (as reported in #45520), and more generally in any environment where the
flash_attnimport is resolvable via a non-pip source,packages_distributions()has no"flash_attn"key. Because the list comprehension is evaluated before theinoperator, short-circuit evaluation of the outeranddoes not protect us, theKeyErrorfires duringtransformersimport and takes down the whole process before any model is loaded.Swap the four raising subscripts for
.get(name, []). If the name is missing from the distribution map we simply conclude that the requested flash-attention flavour is not properly installed, which is the answeris_flash_attn_*_available()would have returned anyway, instead of raising. The inner helper_is_package_availablealready wraps the same subscript in atry/except, so we are only making the outer call sites match that contract.